Skip to content

fix(install): ship native fbuild as a raw wheel script (closes #746)#747

Merged
zackees merged 1 commit into
mainfrom
fix/746-drop-python-shim
Jun 22, 2026
Merged

fix(install): ship native fbuild as a raw wheel script (closes #746)#747
zackees merged 1 commit into
mainfrom
fix/746-drop-python-shim

Conversation

@zackees

@zackees zackees commented Jun 22, 2026

Copy link
Copy Markdown
Member

Summary

  • Drop the [project.scripts] fbuild = ci.bin_launcher:main Python shim and ship the cargo-built native binary directly via setuptools' scripts= mechanism (fbuild-<ver>.data/scripts/fbuild.exe in the wheel).
  • Override build_scripts.copy_scripts so it byte-copies the binary instead of trying to detect a Python coding cookie (SyntaxError).
  • Delete ci/bin_launcher.py and drop ci from [tool.setuptools] packages + [tool.setuptools.package-data].

Closes #746.

Why

On Windows, [project.scripts] generates a pip console-script .exe that imports ci.bin_launcher.main, which os.execvs the bundled native binary. POSIX execv replaces the process; Windows os.execv is emulated as CreateProcess + parent exit, so the Python shim returned to cmd.exe before the spawned Rust binary finished flushing stdout — the next shell prompt then raced ahead of fbuild port scan's output. Same mechanism ci/publish.py already uses for the release wheels on PyPI; only the local-install path was wrong.

Test plan

  • pip install --force-reinstall . produces Scripts/fbuild.exe that file reports as PE32+ executable (not Zip archive console-script stub).
  • cmd /c 'echo === BEFORE === & fbuild port scan & echo === AFTER ===' prints === AFTER === after the scan output, not before.
  • fbuild --version / fbuild --help still work.
  • from fbuild.api import SerialMonitor still imports (PyO3 bindings unaffected).
  • Wheel layout (unzip -l dist/*.whl) shows fbuild-2.2.31.data/scripts/fbuild.exe plus fbuild/, fbuild/api/ packages, and dist-info — no ci/ entries.
  • CI green on Linux + macOS + Windows.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Streamlined the distribution process by directly packaging the native binary without a Python wrapper.
    • Updated build configuration to optimize installation behavior and consistency across platforms.

Previously `pip install .` installed `fbuild` on PATH as a Python
console-script .exe that imported `ci.bin_launcher.main`, which `os.execv`'d
the real cargo-built binary bundled at `ci/bin/fbuild.exe`. On POSIX execv
replaces the process; on Windows it is emulated as `CreateProcess` + parent
exit (no true exec), so the Python shim returned to cmd.exe before the
spawned native binary finished flushing stdout — the next shell prompt then
raced ahead of `fbuild port scan`'s output.

Drop the shim. Ship the cargo-built binary directly via setuptools'
`scripts=` argument in setup.py. Files in `scripts=` land in
`<name>-<version>.data/scripts/` in the wheel, which pip extracts straight
into the install's Scripts/ (Windows) or bin/ (POSIX) directory as-is —
`.exe` files are not wrapped. Result: `cmd → fbuild.exe → stdout`, no
Python in the chain. Same mechanism `ci/publish.py` already uses for
release wheels on PyPI; only the local-install path was broken.

Stock distutils `build_scripts.copy_scripts` tries to detect a Python
coding cookie via `tokenize.open(script)`, which raises SyntaxError on a
PE / ELF binary. Add a `BuildBinaryScripts` subclass that does a byte-level
`shutil.copy2` instead.

- pyproject.toml: drop `[project.scripts] fbuild = ci.bin_launcher:main`,
  drop `ci` from `[tool.setuptools] packages`, drop
  `ci = ["bin/fbuild*"]` package-data entry.
- setup.py: add `scripts=[STAGED_BINARY_PATH]` to setup() and register
  `BuildBinaryScripts` as the `build_scripts` cmdclass.
- Delete `ci/bin_launcher.py` (no longer reachable).

Verified on Windows: `file $(where fbuild)` now reports PE32+ executable
(was Zip-archive console-script stub), and
`cmd /c 'echo === BEFORE === & fbuild port scan & echo === AFTER ==='`
prints `=== AFTER ===` after the scan output instead of before it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a5f226b7-a3b6-467d-a290-d7dc8f2348e2

📥 Commits

Reviewing files that changed from the base of the PR and between fd67061 and 7864eb2.

📒 Files selected for processing (3)
  • ci/bin_launcher.py
  • pyproject.toml
  • setup.py

📝 Walkthrough

Walkthrough

Removes ci/bin_launcher.py (the Python shim that os.execv-ed into the staged Rust binary) and replaces the [project.scripts] console-script entry point with a setup.py-based raw wheel script delivery. A new BuildBinaryScripts command subclasses build_scripts to byte-copy the staged binary directly into the venv bin//Scripts/ directory without generating a Python wrapper.

Changes

Native Binary Distribution

Layer / File(s) Summary
Remove console-script entry point and ci package
pyproject.toml
Replaces [project.scripts] fbuild = "ci.bin_launcher:main" with comments, removes ci from [tool.setuptools].packages, and drops the ci = ["bin/fbuild*"] package-data entry.
BuildBinaryScripts command and setup() wiring
setup.py
Updates module docstring, reorders imports (setuptools before distutils), adds BuildBinaryScripts(build_scripts) with a copy_scripts override that byte-copies binaries (using dep_util or mtime fallback), and registers it via cmdclass with scripts=[str(STAGED_BINARY_PATH)] in setup().

Sequence Diagram(s)

sequenceDiagram
  participant pip
  participant BuildWithCargo
  participant BuildBinaryScripts
  participant venv_bin as venv bin/ or Scripts/

  pip->>BuildWithCargo: build_py — cargo build, stage fbuild into ci/bin/
  BuildWithCargo-->>pip: STAGED_BINARY_PATH available
  pip->>BuildBinaryScripts: build_scripts phase — copy_scripts()
  BuildBinaryScripts->>BuildBinaryScripts: up_to_date check (dep_util / mtime)
  BuildBinaryScripts->>venv_bin: byte-copy fbuild[.exe]
  venv_bin-->>pip: native binary on PATH, no Python shim
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • FastLED/fbuild#458: Also modifies setup.py to build, locate, and stage the compiled Rust fbuild executable into ci/bin/ for wheel installation — directly feeds the path this PR consumes via STAGED_BINARY_PATH.

Poem

🐇 No more Python in the middle lane,
The Rust binary runs, no shim to chain!
execv on Windows? A race we lost —
Now fbuild.exe lands straight, no extra cost.
One hop, not two — the prompt waits its turn,
A native path forward, hard-won and firm! 🥕

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/746-drop-python-shim

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@zackees zackees merged commit a426a0d into main Jun 22, 2026
83 of 91 checks passed
@zackees zackees deleted the fix/746-drop-python-shim branch June 22, 2026 05:29
zackees added a commit to zackees/template-python-rust-cmd that referenced this pull request Jun 22, 2026
…#2 items 1+10) (#7)

Previously the Python entry point `[project.scripts]
template-python-rust-cmd = "template_python_rust_cmd.cli:main"` created
a pip console-script `.exe` that subprocess-launched the staged
`_bin/template-cli`. On Windows that pattern races the next shell
prompt ahead of the child's stdout, because the Python shim returns
to cmd.exe before the spawned native binary finishes flushing. Same
class of bug fbuild fixed in FastLED/fbuild#747.

Drop the shim. ci/build_wheel.py now post-processes the maturin-built
wheel: opens the zip, injects the cargo-built `template-cli[.exe]` at
`<name>-<ver>.data/scripts/template-cli[.exe]`, recomputes the RECORD
row (sha256 + size), and stamps the Unix executable bit on the entry.
Files in `.data/scripts/` are pip's canonical raw-script install
location — pip drops them straight into the venv's `Scripts/` (Win) /
`bin/` (POSIX) directory verbatim, with no Python wrapper for `.exe`
files. Same mechanism cargo-dist and maturin's "bin" mode use.

Removed:
- `[project.scripts]` table from pyproject.toml.
- `src/template_python_rust_cmd/cli.py` (the racy subprocess shim).
- `src/template_python_rust_cmd/_bin/.gitkeep` + the gitignore lines
  that staged binary into the package data directory.
- `action/cleanup/action.yml` step that rm'd the (now-nonexistent)
  `_bin/` staging directory.

Verified end-to-end on Windows:
- Fresh maturin wheel produced via `uv run --no-project --script
  ci/build_wheel.py`.
- `unzip -l dist/*.whl` shows `template_python_rust_cmd-0.1.0.data/
  scripts/template-cli.exe` (1.2 MB) — no `_bin/` entries.
- `uv pip install` of that wheel into a fresh venv lands a `Scripts/
  template-cli.exe` that `file` reports as `PE32+ executable` (NOT a
  Zip-archive console-script stub).
- `cmd /c 'echo === BEFORE === & template-cli.exe --version & echo
  === AFTER ==='` prints `=== AFTER ===` AFTER the version output —
  acceptance criterion from issue #2 met.

The `no-build-isolation-package` setting added in PR #5 was reverted
in this PR: with maturin (vs. setuptools) the trade-off doesn't favor
us — uv reuses a stale build env that lacks maturin, and the preview
`extra-build-dependencies` mechanism is too fragile to ship in a
template. The CARGO_TARGET_DIR pin from that PR stays — it already
gives the cargo-incremental win without touching isolation.

Closes #2 (items 1 + 10).
Refs FastLED/fbuild#747.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@zackees zackees mentioned this pull request Jun 22, 2026
2 tasks
zackees added a commit that referenced this pull request Jun 22, 2026
Minor bump. Picks up:
- #747 native fbuild as raw wheel script (drops Python launcher; fixes
  Windows stdout-ordering bug on COM25     303A:1001    USB Serial Device (COM25)    ser=80:F1:B2:D1:DF:B1
          └─ Espressif Systems / ESP32-S3 USB-CDC

COM1      [Unknown]
          └─ (no USB identifier — Unknown endpoint)

COM3      [Unknown]
          └─ (no USB identifier — Unknown endpoint)

COM22     303A:1001    USB Serial Device (COM22)    ser=D8:3B:DA:41:64:C0
          └─ Espressif Systems / ESP32-S3 USB-CDC

COM4      [Unknown]
          └─ (no USB identifier — Unknown endpoint)

COM9      303A:1001    USB Serial Device (COM9)    ser=8C:BF:EA:CF:87:B4
          └─ Espressif Systems / ESP32-S3 USB-CDC

COM20     16C0:0483    USB Serial Device (COM20)    ser=15821020
          └─ Van Ooijen Technische Informatica / Teensyduino Serial

COM10     1FC9:0132    USB Serial Device (COM10)    ser=0B03400A
          └─ NXP Semiconductors / LPC-Link2 CMSIS-DAP

5 USB ports, 3 non-USB ports)
- #748 src-layout package-dir (static analyzers resolve fbuild.api)
- #750 daemon WS handler split into reader/writer/inbound
- #751 gitignore .extern-repos/
- #752 cargo fmt + extract test modules so 4 files fall under the
  1000-LOC gate

No public-API changes beyond what's already documented in the merged
PRs above.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
@fastled-project-sync fastled-project-sync Bot moved this to Triage in FastLED Tracker Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

Ship fbuild as a raw native binary, drop the Python wrapper (cmd → fbuild.exe directly)

1 participant